Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Better logging when mining own transactions. #9363

Merged
merged 1 commit into from
Aug 17, 2018
Merged

Conversation

tomusdrw
Copy link
Collaborator

Currently we log Transaction is mined for every local (own) transaction that is dropped from the queue.
In fact it's not always true, if the same key is used on some other nodes or if we send multiple replacement transactions (bumping gas price, but using the same nonce) - the transaction might be removed, but some other transaction with the same (sender, nonce) was actually included in the blockchain.

This PR attempts to fix the logging for local transactions by checking if the transaction is in chain before printing the log line.

Since we don't have a client instance in the miner, we do that by using a Weak reference (to prevent cyclic references).

@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Aug 16, 2018
Copy link
Collaborator

@sorpaas sorpaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ascjones ascjones merged commit 346913b into master Aug 17, 2018
@ascjones ascjones deleted the td-txlistener branch August 17, 2018 15:01
@5chdn 5chdn added this to the 2.1 milestone Aug 23, 2018
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants